Use enum values from enumType in DiscriminatorColumn and check DiscriminatorMap values against it#12065
Conversation
|
What's the correct way to turn off tests for DBAL versions not supporting enumType? |
|
You could mark the test as skipped unless |
|
Let's try this now. |
whataboutpereira
force-pushed
the
fix-enum-discriminator-column
branch
from
9ad4a23 to
fa84570
Compare
|
MySQL also required. :) |
whataboutpereira
force-pushed
the
fix-enum-discriminator-column
branch
from
fa84570 to
15bb988
Compare
|
Sorry, that was depressing. Last attempt for now. |
|
Retargeting to 3.6.x since this is a new feature. |
| } | ||
|
|
||
| if (! $this->getTestEntityManager()->getConnection()->getDatabasePlatform() instanceof AbstractMySQLPlatform) { | ||
| self::markTestSkipped('Test valid for MySQL/MariaDB only.'); |
There was a problem hiding this comment.
Why shouldn't we run this test on other DBMS?
There was a problem hiding this comment.
Tests were failing on SQLite. I'm not sure which other feature detection we should use to skip.
There was a problem hiding this comment.
The more I look at it it the more confused I get. :) Now I'm thinking Types::ENUM doesn't need to be involved at all when setting the values in ClassMetadata, because you should still get the values from the enum class even if not using type enum in the database. But it's 2AM, will test tomorrow.
whataboutpereira
force-pushed
the
fix-enum-discriminator-column
branch
2 times, most recently
from
f4617f5 to
689a5eb
Compare
|
|
|
Things got a bit clearer. It seems I was mislead by the title "Implement an EnumType for MySQL/MariaDB", where in fact it works for all databases, but just maps the value to string. So Types::ENUM should always be used to populate values. |
whataboutpereira
force-pushed
the
fix-enum-discriminator-column
branch
from
76dcaee to
b84e46a
Compare
|
Can you please squash your commits together and give the remaining commit a good message? |
whataboutpereira
force-pushed
the
fix-enum-discriminator-column
branch
from
b84e46a to
6da5cc4
Compare
Thanks, done now. Hopefully correctly. :) |
| ); | ||
| } | ||
|
|
||
| $values = $this->discriminatorColumn->options['values'] ?? null; |
There was a problem hiding this comment.
I'm now thinking we should wrap these checks in if ($this->discriminatorColumn?->enumType !== null), otherwise DiscriminatorColumn without enumType, but with options: ['values' => ['...']] will fail. Though values probably shouldn't be used without.
whataboutpereira
force-pushed
the
fix-enum-discriminator-column
branch
from
6da5cc4 to
1660384
Compare
|
Okay, I rebased against v3.5 which is behind the v3.6 that I should have used. Fixed again. |
whataboutpereira
force-pushed
the
fix-enum-discriminator-column
branch
from
1660384 to
c7aa436
Compare
Check DiscriminatorMap keys match enum cases. Test values are populated from enum cases and mismatched values throw an exception. Fixes #11794
whataboutpereira
force-pushed
the
fix-enum-discriminator-column
branch
from
c7aa436 to
6f83166
Compare
|
Thanks @whataboutpereira ! |
Thank you as well! :) |
4 participants
Fix for issue #11794.
Currently the values of enumType in DiscriminatorColumn are ignored which results in an error when generating a migration:
Fixed by populating values from the enum and also prevent invalid entries in DiscriminatorMap.